Prove collect-rewards against the head, not the finalized block - #152
Conversation
…zed block QPoW finality trails the head by a long way (~100 blocks on staging mainnet), so any leaf minted inside that window is absent from the ZK tree at the finalized block. collect_rewards proved against chain_getFinalizedHead, which made recent rewards unsweepable and failed with "Leaf index N not found in ZK tree at block H" part-way through proof generation. Prove against the head instead, matching every other read path in the CLI. A proof invalidated by a reorg is re-run, which is far cheaper than waiting for finality. --at-block still pins a specific block for callers who want one. Also check settlement before generating proofs rather than discovering it on the Nth one: leaves settle in index order, so one probe of the highest selected leaf covers the common case and a binary search finds the boundary otherwise. Leaves that are still too new are now reported and skipped instead of aborting the whole sweep, and the all-unsettled case gets an error that says what to do about it. Splits get_zk_merkle_proof into try_get_zk_merkle_proof, which returns Ok(None) for an unsettled leaf so callers can tell that apart from an RPC failure.
n13
left a comment
n13
left a comment
There was a problem hiding this comment.
Reviewer model: GPT Sol
Verdict (advisory): Request changes
Blocking findings:
-
src/collect_rewards_lib.rs:434filters unsettled leaves only after the--amountselection at lines 351-359 has stopped and discarded the remaining candidates. If a high-value recent leaf is selected first, this can now error with no provable transfers even though an omitted settled transfer could satisfy the request; with a mixed selection it can submit less than the requested amount and still report success. For example, a requested amount of 50 with an unsettled 100 transfer and an older settled 60 transfer selects only the 100 transfer, removes it here, and errors instead of withdrawing the available 60. Filter all unspent transfers to the provable prefix before amount selection, or backfill from settled unselected transfers and revalidate the requested amount after filtering. -
The exact head is not formatted with the repository-pinned toolchain.
cargo +nightly-2026-08-31 fmt --all -- --checkreports diffs in both changed files (src/cli/wormhole.rs:204,src/collect_rewards_lib.rs:59, andsrc/collect_rewards_lib.rs:369). The live Fast Checks job fails on the same diffs, so every downstream CI job is skipped. Please apply the pinned formatter.
Validation on exact head 471536571d3af6866a2bd447c542cdae428f73c1:
git diff --check— passed.taplo format --check --config taplo.toml— passed.cargo +nightly-2026-08-31 fmt --all -- --check— failed as described above.SKIP_CIRCUIT_BUILD=1 cargo test --release --locked --lib collect_rewards_lib::tests— 15 passed.SKIP_CIRCUIT_BUILD=1 cargo clippy --release --all-targets --locked -- -D warnings— passed.- Full release library run compiled and passed 323 tests; its sole failure was the pre-existing artifact-dependent verifier test because the isolated review run intentionally skipped circuit-bin generation.
No test in this patch exercises settlement filtering together with --amount; please add coverage for backfilling/amount preservation when the initially selected high-value leaf is unsettled.
The settlement filter ran after the --amount selection, so a large not-yet-settled leaf could be selected, then dropped, leaving the sweep short of the requested amount or failing outright while an older settled transfer could have covered it. Establish the settlement boundary before selecting: resolve the proof block, probe the boundary over all unspent transfers, then run the largest-first selection over the settled ones only. The requested amount is validated against the provable total, and the shortfall error says how many transfers were held back as too new. Also format with the pinned nightly rustfmt.
n13
left a comment
n13
left a comment
There was a problem hiding this comment.
Reviewer model: GPT Sol
Verdict (advisory): Approve
No blocking findings.
The follow-up commit resolves both earlier blockers. Settlement is now determined across all unspent leaf indices before largest-first amount selection, so a recent unsettled leaf cannot crowd an older provable transfer out of the withdrawal set. The requested amount is rechecked against settled funds, with focused coverage for backfilling and an insufficient settled balance. The pinned formatting failure is also resolved.
I traced the fixed-block proof path into the node/runtime implementation as well: zkTree_getMerkleProof honors the supplied block hash, and the runtime exposes a monotonic processed-leaf prefix, which supports the boundary search used here. The existing public get_zk_merkle_proof error behavior remains intact through the new optional wrapper.
Validation on exact head 2932cd1c47289711a6cbd049ca8419be422c133f:
git diff --check- passed.taplo format --check --config taplo.toml- passed.cargo +nightly-2026-08-31 fmt --all -- --check- passed.cargo metadata --locked --no-deps --format-version 1- passed.SKIP_CIRCUIT_BUILD=1 cargo test --release --locked --lib collect_rewards_lib::tests- 19 passed.SKIP_CIRCUIT_BUILD=1 cargo clippy --release --all-targets --locked -- -D warnings- passed.- Hosted CI - all current checks passed, including Ubuntu/macOS builds and tests, examples, strict analysis/docs, formatting, security audit, and dependency cooldown.
1 participant
Problem
collect-rewardsfailed on staging mainnet with:The chain is QPoW, so finality trails the head by a long way — measured at ~100 blocks on staging mainnet.
collect_rewardsproved againstchain_getFinalizedHead, so any leaf minted inside that window is not yet in the tree at the proof block.Confirmed against the live chain: the failing block
0x4d5336…is block 1730, and leaf 1780 did not settle until block 1750.getMerkleProof(1780, 1730)returns null;getMerkleProof(1780, 1750)returns a proof. Nothing was corrupt — the leaf simply did not exist yet at the block being proved against.It also worked on the dev chain, which finalizes immediately, so the window never opens there.
Changes
Prove against the head. Every other read path in the CLI already uses the latest block; this one was the outlier. The existing comment was right that a best-block proof can be reorged out, but on PoW the answer to that is re-running the proof, not waiting ~100 blocks.
--at-blockstill pins a specific block.Check settlement before generating proofs. The run above died on proof 66 of 1809 after 107 seconds. Leaves settle in index order, so provability is monotonic: one probe of the highest selected leaf covers the common case, and a binary search finds the boundary otherwise. Same outcome, about a second.
Warn and skip instead of aborting. Leaves that are still too new are reported and dropped so one recent transfer cannot hold up an entire sweep. If nothing is provable, the error now says why and what to do.
try_get_zk_merkle_proof. ReturnsOk(None)for an unsettled leaf so callers can tell that apart from an RPC failure.get_zk_merkle_proofis now a thin wrapper over it, so the RPC call is not duplicated.Testing
cargo build --releasecleancargo clippy --release --all-targetscleancargo test --release --lib— 324 passed, 0 failedBehaviour verified against the live staging chain by direct RPC (block-by-block binary search for the settlement boundary of leaf 1780) rather than by re-running the sweep.
Not included
Two related issues live in other repos and are deliberately out of scope:
runtime/src/apis.rscollapsesLeafIndexOutOfBoundsandLeafNotYetSettledinto a singleNone, which is why the original error could not say which had happened; andzkTree_getStatetakes no block argument while silently accepting one.